fix(sqlparser): typed string must be single quoted#17482
Conversation
| formatted_sql: SHOW TABLES FROM t | ||
| formatted_ast: 'ShowObjects { object: Table { schema: Some(Ident { value: "t", quote_style: None }) }, filter: None }' | ||
| - input: SHOW TABLES FROM t LIKE "t%" | ||
| - input: SHOW TABLES FROM t LIKE 't%' |
There was a problem hiding this comment.
Strictly speaking this fix is a breaking change. But it was wrong syntax and our documentation has been using single quotes correctly. The impact shall be negligible.
|
This risingwave/src/sqlparser/src/ast/mod.rs Lines 2885 to 2890 in 18f9980 cc @wangrunji0408 Shall we introduce a new variant for (unquoted or double quoted) identifier? |
Signed-off-by: Runji Wang <wangrunji0408@163.com>
added Actually I think the string after |
| alt(( | ||
| single_quoted_string.map(FunctionDefinition::SingleQuotedDef), | ||
| dollar_quoted_string.map(FunctionDefinition::DoubleDollarDef), | ||
| Self::parse_identifier.map(|i| FunctionDefinition::Identifier(i.value)), |
There was a problem hiding this comment.
When I said (unquoted or double quoted) identifier I was thinking of .real_value() but now I realize .value has better backward compatibility:
When the SQL to parse is as camelCase, the old behavior gets camelCase but the identifier's real_value is camelcase. Although as "camelCase" is a case-sensitive identifier, I agree it is better to encourage as 'camelCase' if breaking change is inevitable.
Signed-off-by: Runji Wang <wangrunji0408@163.com> Co-authored-by: Runji Wang <wangrunji0408@163.com>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
See #17461. Somehow a user wrote the invalid SQL
Date t::datebut it was interpreted by our sqlparser as(Date 't')::dateand resulting in evaluation error.The root cause is upstream apache/datafusion-sqlparser-rs#235 support of HiveSQL where an unquoted identifier can be treated as a literal string value. It is never the case for PostgreSQL.
After the fix, we report the following error message:
which is similar to PostgreSQL:
Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.